Improve Lighthouse Scrore#6296
Conversation
Merging this PR will not alter performance
Comparing 
Footnotes
|
Greptile SummaryThis PR improves Lighthouse scores through five coordinated changes: (1) build-time pre-compression of static assets (gzip/brotli/zstd) via a new Vite plugin and
Confidence Score: 4/5Safe to merge for most users; Tailwind v4 users will see a regression where Tailwind CSS is not injected after the granular Radix import path change. One P1 finding: the _RADIX_IMPORT_RE regex in shared_tailwind.py uses url(...) syntax but Tailwind v4's new template emits bare @import "…" layer(components), so add_tailwind_to_css_file never injects Tailwind for v4 users. The remaining findings are P2. All other new functionality is well-implemented and well-tested. packages/reflex-base/src/reflex_base/plugins/shared_tailwind.py — _RADIX_IMPORT_RE regex needs to cover the v4 bare @import format Important Files Changed
Sequence DiagramsequenceDiagram
participant App as reflex build
participant Vite as Vite + Plugins
participant FS as Static FS
participant ASGI as PrecompressedStaticFiles
participant Browser
App->>Vite: vite build
Vite->>FS: write JS/CSS/HTML chunks
Vite->>FS: compressPlugin → write .gz/.br sidecars
Vite->>FS: imageOptimizePlugin → write .webp/.avif sidecars
App->>FS: duplicate route index.html + copy sidecars
App->>FS: copy SPA fallback → 404.html + sidecars
App->>FS: _compress_static_output (no-op if Vite already ran)
Browser->>ASGI: GET /app.js (Accept-Encoding: br, gzip)
ASGI->>FS: lookup app.js.br
FS-->>ASGI: found
ASGI-->>Browser: 200 app.js.br (Content-Encoding: br, Vary: Accept-Encoding)
Browser->>ASGI: GET /hero.png (Accept: image/avif, image/webp)
ASGI->>FS: lookup hero.avif
FS-->>ASGI: found
ASGI-->>Browser: 200 hero.avif (Content-Type: image/avif, Vary: Accept)
Reviews (2): Last reviewed commit: "fix: expose dynamic component tags on wi..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| try: | ||
| subprocess.run( | ||
| command, | ||
| check=True, | ||
| capture_output=True, | ||
| text=True, | ||
| timeout=180, | ||
| ) | ||
| except subprocess.CalledProcessError as err: | ||
| pytest.fail( | ||
| "Lighthouse execution failed.\n" | ||
| f"Command: {' '.join(command)}\n" | ||
| f"stdout:\n{err.stdout}\n" | ||
| f"stderr:\n{err.stderr}" | ||
| ) |
There was a problem hiding this comment.
subprocess.TimeoutExpired not handled
subprocess.run() is called with timeout=180, but only CalledProcessError is caught. If Lighthouse takes longer than 180 seconds, subprocess.TimeoutExpired will propagate uncaught, producing a raw Python traceback in CI output rather than a friendly pytest.fail() message.
| try: | |
| subprocess.run( | |
| command, | |
| check=True, | |
| capture_output=True, | |
| text=True, | |
| timeout=180, | |
| ) | |
| except subprocess.CalledProcessError as err: | |
| pytest.fail( | |
| "Lighthouse execution failed.\n" | |
| f"Command: {' '.join(command)}\n" | |
| f"stdout:\n{err.stdout}\n" | |
| f"stderr:\n{err.stderr}" | |
| ) | |
| try: | |
| subprocess.run( | |
| command, | |
| check=True, | |
| capture_output=True, | |
| text=True, | |
| timeout=180, | |
| ) | |
| except subprocess.TimeoutExpired as err: | |
| pytest.fail( | |
| f"Lighthouse timed out after {err.timeout}s.\n" | |
| f"Command: {' '.join(command)}" | |
| ) | |
| except subprocess.CalledProcessError as err: | |
| pytest.fail( | |
| "Lighthouse execution failed.\n" | |
| f"Command: {' '.join(command)}\n" | |
| f"stdout:\n{err.stdout}\n" | |
| f"stderr:\n{err.stderr}" | |
| ) |
- Wrap blank template in rx.el.main landmark with page title/description - Add aria_label/title to ColorModeIconButton and StickyBadge - Add landing page prod benchmark alongside blank template test - Document page structure and metadata best practices
Add a Vite build plugin that generates .gz copies of assets for sirv to serve pre-compressed. Split socket.io and radix-ui into separate chunks for better caching. Refactor lighthouse benchmark to use `reflex run --env prod` directly instead of AppHarnessProd.
|
Shouldn't the compression be left to the reverse-proxy/ingress/deployment? If not, consider adding brotli alongside gzip? |
Yes, generally. However, better performance can be achieved by configuring the reverse proxy to serve the pre-compressed files so they don't need to be compressed on the fly when requested. @FarhanAliRaza a good point is raised here though, I think the vite compress plugin should be able to handle output to gzip, brotli, or zstd for completeness. We should add some documentation to the self-hosting page about how to enable and use pre-compressed responses in caddy and nginx. The other thing is that I found this old, possibly unmaintained starlette plugin: https://github.com/magnuswatn/starlette-precompressed-static It pretty much does what we want with the exception that it doesn't support Because compression does touch on the intersection between application and deployment, we need to make sure these options are configurable by the end user. I don't think anyone is directly reverse proxying to a |
…ic assets Support gzip, brotli, and zstd build-time pre-compression via the new `frontend_compression_formats` config option. The Vite compress plugin now handles multiple formats, and a new PrecompressedStaticFiles middleware negotiates Accept-Encoding to serve matching sidecar files directly. Replace the custom http.server-based prod test harness with Starlette/Uvicorn to match the production serving stack. Remove redundant post-build compression pass that was re-compressing assets already handled by the Vite plugin. Move sidecar stat calls off the async event loop into a worker thread.
…lities - Extract shared walkFiles/outputDirectoryExists/validateFormats into vite-plugin-utils.js - Add vite-plugin-image-optimize for WebP/AVIF sidecar generation - Add vite-plugin-purgecss to strip unused CSS from radix-ui - Add bounded concurrency to compression plugin - Simplify format normalization: config validates, downstream just looks up - Fix sidecar copy only running when target HTML was actually created - Remove blank page lighthouse test, keep only landing page - Add integration test for full prod build pipeline (gz, purge, image opt)
# Conflicts: # pyi_hashes.json # reflex/app.py # tests/units/test_prerequisites.py
FarhanAliRaza
force-pushed
the
lighthouse-improve
branch
from
3b6eef0 to
11aae47
Compare
- Run compress-static.js on final static output to pre-compress assets - Skip already-compressed sidecars and always compress HTML entrypoints - Switch socket.io decoder to JSON5.parse for more lenient message parsing - Add json5 as a frontend dependency
Replace StaticFiles with PrecompressedStaticFiles in get_frontend_mount to support content negotiation for compressed and optimized image formats. Add unit tests for the new behavior.
# Conflicts: # packages/reflex-base/src/reflex_base/compiler/templates.py # reflex/testing.py # reflex/utils/exec.py
Add node-version input to setup_build_env action instead of hardcoding v22. Include actual timeout duration in Lighthouse CLI failure messages and add unit tests for both preparation and execution timeout paths.
Only ship the Radix color scales actually referenced by Theme components (falling back to the monolithic stylesheet when a color is state-driven), and expose external libraries on window.__reflex via named imports so Rolldown can drop unused exports.
Capture named imports during Component serialization so tags only reachable via evalReactComponent (Component-typed vars, state field defaults) are included in window.__reflex. Without this, dynamic components fail to resolve their imports after the tree-shaking changes. Also splits @radix-ui/themes into its own chunk.
FarhanAliRaza
force-pushed
the
lighthouse-improve
branch
from
e7f9c2a to
31820ec
Compare
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
# Conflicts: # packages/reflex-base/src/reflex_base/plugins/base.py # packages/reflex-base/src/reflex_base/plugins/tailwind_v3.py # packages/reflex-base/src/reflex_base/plugins/tailwind_v4.py # reflex/app.py # reflex/compiler/compiler.py # tests/units/compiler/test_compiler.py
# Conflicts: # packages/reflex-base/src/reflex_base/plugins/base.py # packages/reflex-base/src/reflex_base/plugins/tailwind_v3.py # packages/reflex-base/src/reflex_base/plugins/tailwind_v4.py # reflex/app.py # reflex/compiler/compiler.py # tests/units/compiler/test_compiler.py
…/reflex into lighthouse-improve
masenf
left a comment
masenf
left a comment
There was a problem hiding this comment.
quite a bit of feedback here.
my suggestion would be to break up this PR into smaller chunks so we can bring it in incrementally.
i'd probably start with the test infrastructure for running lighthouse in CI, then bring each component in a separate PR so we can see the effect each change has on the lighthouse score in the test.
for particularly hairy changes or ones that might have edge cases, we can make an informed determination whether the added complexity is worth the perf/accessibility/seo improvement on the test rubric.
| const chunk = JSON5.parse(chunk_json); | ||
| const chunk = JSON.parse(chunk_json); |
There was a problem hiding this comment.
can this reuse the parsing method that the main websocket dispatch loop is using? these chunks could be anything from the backend, including weird floats
| underline="none", | ||
| ), | ||
| as_child=True, | ||
| high_contrast=True, |
There was a problem hiding this comment.
do these changes actually affect the lighthouse score?
| # Captured during Component serialization so ``collect_window_library_imports`` | ||
| # can expose tags reachable only through eval'd code on ``window.__reflex``; | ||
| # without this, ``evalReactComponent`` would fail to resolve them. | ||
| dynamic_component_imports: dict[str, set[imports.ImportVar]] = {} |
There was a problem hiding this comment.
we should probably move this to RegistrationContext (I already have #6382 moving some other globals over). The idea with RegistrationContext is that all of the "globals" associated with an App can live there, allowing two different apps to exist with their own config, etc in the same process.
| # Captured during Component serialization so ``collect_window_library_imports`` | ||
| # can expose tags reachable only through eval'd code on ``window.__reflex``; | ||
| # without this, ``evalReactComponent`` would fail to resolve them. | ||
| dynamic_component_imports: dict[str, set[imports.ImportVar]] = {} |
There was a problem hiding this comment.
I'm also slightly concerned that this might miss dynamic components that are constructed at runtime in event handlers -- the compiler only sees these as initial_state, which may not follow conditional paths causing some other imports to be needed.
Ideally, if the user specifically calls bundle_library, that library should always get bundled into the frontend regardless of if it's used anywhere
| for root in roots: | ||
| if root is None: | ||
| continue | ||
| for node in _walk_components(root): |
There was a problem hiding this comment.
i think we need to update this to plug into the new compiler tree walk now that it's merged in
| if root is None: | ||
| continue | ||
| for node in _walk_components(root): | ||
| if getattr(node, "tag", None) != "Theme": |
There was a problem hiding this comment.
other components besides Theme can reference theme colors; most radix components have mapped the reflex color_scheme prop to the underlying radix themes color prop which accepts a color name. We also have the rx.color helper which takes radix themes color names and translates them into CSS for use with any component. Not clear if those cases are also handled here.
| self.frontend_server.serve_forever() | ||
| config = get_config() | ||
| frontend_app = Starlette() | ||
| frontend_app.mount( |
There was a problem hiding this comment.
for prod mode, i don't think we even need to do this anymore. if we just run the prod backend without backend_only mode, then we'll get this kind of behavior out of the backend server and it will more closely resemble how the app actually runs in prod mode.
if we can avoid drift between how the framework really runs and how the test harness runs, we'll be more likely to catch issues during testing.
3 participants
All Submissions:
Type of change
Please delete options that are not relevant.
New Feature Submission:
Changes To Core Features:
closes #6295